Skip to content

Harden status recovery and add staged toolbar updates - #7

Open
Vivek-Raman wants to merge 1 commit into
mainfrom
fix/codebase-remediation
Open

Vivek-Raman wants to merge 1 commit into
mainfrom
fix/codebase-remediation

Conversation

@Vivek-Raman

Copy link
Copy Markdown
Owner

Sideglance could freeze during filesystem and agent CLI operations, lose user data during interrupted updates, and show stale reports without explaining their state. This change makes status storage recoverable, moves blocking operations off the UI thread, and adds an explicit check → download → install flow to the main toolbar.

Changes

  • Keep user pins in private storage while preserving the shared v2 format for older MCP writers. Journal pin changes, preserve newer legacy writes during recovery, and back up inline-pin data before migration.
  • Archive automatically removed reports before deletion, with bounded history and restore-and-pin actions. Preview destructive retention changes before applying them; recover invalid settings from preserved originals.
  • Bound lock and subprocess waits, coalesce watcher events, suppress unchanged snapshots, and expose waiting/stale/error states. Add persistent collapsed groups, session attribution, and selection pause/resume without shifting controls under the pointer.
  • Stage skill replacements with retained backups; detect intervening agent-config edits. Add read-only MCP connection testing and repair actions.
  • Replace the toolbar update indicator with separate icon actions for checking, downloading, and installing. Download/install actions remain visible when the window loses focus; progress, retries, resource cleanup, and restart are handled explicitly.
  • Lazy-load settings, pause decorative SVG playback when inactive, reduce MCP runtime threads, and strip release symbols. Upgrade frontend tooling and gate release builds on reusable PR checks, including native Rust checks on macOS, Linux, and Windows.

Validation

  • 86 Rust tests passed, including interrupted-write recovery and a local mixed-version test using the preserved older MCP executable.
  • 52 frontend unit tests and 20 Playwright tests passed.
  • Svelte/TypeScript checks, frontend and release builds, formatting, strict Clippy, and diff whitespace checks passed.
  • Temporary-home MCP smoke tests and macOS executable signature verification passed. Tests did not install updates or alter live agent configurations.

Limits and rollout

Restart existing agent sessions after upgrading so they load the new MCP executable. Preserve both status files during backup or recovery; automatic-deletion history is bounded rather than permanent storage.

Two low upstream npm build-tool advisories remain with no fix reported by npm; there are no high, moderate, or critical npm findings. RustSec tooling was unavailable locally. Windows/Linux runtime behavior and native window profiling remain for CI/manual verification. Background throttling remains unchanged pending native event-delivery validation.

- Add cross-platform checks and tagged-release verification
- Improve retention, pin recovery, integrations, and viewer health
- Upgrade frontend tooling and document validation workflows

@Vivek-Raman Vivek-Raman left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found two verified P2 issues on modified lines: repeated migration backups during mixed-version operation, and native always-on-top updates that can execute out of settings-commit order. Both findings were independently verified. No other actionable findings met the confidence threshold across correctness, architecture, security, performance, dependency declarations, repository conventions, and history.

.get("points")
.and_then(|v| v.as_array())
.is_some_and(|points| {
points.iter().any(|point| point.get("pinned").is_some())

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Avoid a new migration backup for every false-only legacy report

A still-running pre-PR v2.5 MCP serializes pinned: false on every ordinary report. The new reader treats the field's presence as a migration, so each subsequent legacy report reaches the UUID backup creation below. Two independent temporary-home reproductions produced one additional full-file backup per report, even with no pinned points. These files are never trimmed and can each approach the 1 MiB status limit, bypassing the bounded status/history storage.

Only preserve a migration backup when actual inline pins need preservation, or keep a bounded migration backup. False-only fields can still be stripped without creating another backup:

Suggested change
points.iter().any(|point| point.get("pinned").is_some())
points.iter().any(|point| {
point.get("pinned").and_then(serde_json::Value::as_bool)
== Some(true)
})

Comment on lines +274 to +277
owner.set_pinned(pinned)?;
app.get_webview_window("main")
.ok_or_else(|| "Main window unavailable".to_string())?
.set_always_on_top(pinned)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Order native pin effects with the settings commits

Moving these handlers onto parallel blocking workers allows A to persist true, B to persist false and enqueue the native false update, then A to enqueue true. SettingsOwner releases its mutex before the native call, and the settings checkbox permits overlapping requests. Tauri queues worker-originated native calls independently, so the saved setting and UI can finish unchecked while the viewer remains always on top. The previous synchronous handlers ran serially on the main thread.

Serialize the settings commit and native enqueue through one shared PinStateCoordinator used by both set_pinned and reset_settings, which has the same gap. Reading the latest snapshot before enqueueing does not close the race unless both steps share that ordering boundary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant